Replace wildcard imports with explicit imports in utils and markers#4362
Replace wildcard imports with explicit imports in utils and markers#4362anandheritage wants to merge 3 commits intoisaac-sim:mainfrom
Conversation
…odules - Replaces 'from .module import *' with explicit named imports - Improves code readability and IDE autocompletion - Makes it easier to track which symbols are exported - Follows Python best practices for explicit imports Modified files: - source/isaaclab/isaaclab/utils/__init__.py - source/isaaclab/isaaclab/markers/__init__.py Signed-off-by: Anand Kumar Shaw <anandkumarshaw@users.noreply.github.com>
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR attempts to replace wildcard imports (from module import *) with explicit named imports in the utils and markers modules, following PEP 8 guidelines. However, it contains a critical error:
Critical Issue Found:
The utils module attempts to import four non-existent functions from the mesh submodule:
convert_mesh_to_meshio_meshmake_convex_mesh_from_meshmake_mesh_from_geometrymerge_meshes
These functions are not defined in mesh.py and have never existed in the codebase. The mesh module's __all__ list actually exports only: create_trimesh_from_geom_mesh, create_trimesh_from_geom_shape, convert_faces_to_triangles, and PRIMITIVE_MESH_TYPES.
Positive Notes:
- The markers module replacement is correct - all imported symbols exist and are properly defined
- The approach of listing explicit imports is correct and follows PEP 8
- All other utils module imports appear to be valid (array, buffers, configclass, dict, interpolation, logger, modifiers, string, timer, types, version)
The PR needs to be corrected to import the actual functions exported from the mesh module.
Confidence Score: 0/5
- This PR is NOT safe to merge - it will cause an ImportError at runtime due to non-existent imports from the mesh module.
- The PR introduces a critical syntax/import error that will prevent the utils module from being imported. The four functions being imported from mesh.py do not exist in that module, and this will break any code that tries to import from isaaclab.utils. While the approach and markers changes are correct, this single critical error makes the entire PR non-functional.
- source/isaaclab/isaaclab/utils/init.py requires immediate correction - must replace the non-existent mesh function imports with the actual exports from mesh.py
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab/isaaclab/utils/init.py | 0/5 | Contains critical import errors - attempts to import 4 non-existent functions from mesh module (convert_mesh_to_meshio_mesh, make_convex_mesh_from_mesh, make_mesh_from_geometry, merge_meshes). This will cause an ImportError at runtime. The PR introduces non-existent symbols that break the utils module import. |
| source/isaaclab/isaaclab/markers/init.py | 5/5 | Correctly replaces wildcard imports with explicit imports. All marker configuration imports and VisualizationMarkers classes are properly listed and exist in the source modules. |
Sequence Diagram
sequenceDiagram
participant User as User Code
participant UtilsInit as utils/__init__.py
participant MeshMod as mesh.py
participant MarkersInit as markers/__init__.py
participant ConfigMod as config/__init__.py
User->>UtilsInit: import isaaclab.utils
UtilsInit->>MeshMod: from .mesh import convert_mesh_to_meshio_mesh, ...
MeshMod-->>UtilsInit: ❌ ImportError - functions don't exist
Note over UtilsInit: Correct imports should be:<br/>from .mesh import create_trimesh_from_geom_mesh, ...<br/>(4 existing functions from __all__)
User->>MarkersInit: import isaaclab.markers
MarkersInit->>ConfigMod: from .config import BLUE_ARROW_X_MARKER_CFG, ...
ConfigMod-->>MarkersInit: ✓ Success - all 11 configs exist
MarkersInit->>User: ✓ Success
- Replace non-existent function names with actual mesh.py exports - Use: create_trimesh_from_geom_mesh, create_trimesh_from_geom_shape - Use: convert_faces_to_triangles, PRIMITIVE_MESH_TYPES - Fix identified by Greptile bot review Signed-off-by: Anand Kumar Shaw <anandkumarshaw@users.noreply.github.com>
|
This is my first PR - Landed here after while reading a research paper. Will like to contribute to this community. |
2ef7fc8 to
f3061a4
Compare
|
Congrats on making your first PR. Unforuntately, we would need to close it as currently we do wildcard import everywhere, and it is a bigger project discussion on if we want to change that. Feel free to open a github issue instead for discussions. |
This PR replaces wildcard imports (
from module import *) with explicit named imports in theutilsandmarkersmodules, following PEP 8 guidelines to improve code clarity and maintainability.PEP 8 Guidance on Wildcard Imports
From PEP 8 - Imports: